-
Notifications
You must be signed in to change notification settings - Fork 299
intrinsic-test
: Adding x86 behavioural testing.
#1894
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
intrinsic-test
: Adding x86 behavioural testing.
#1894
Conversation
41db5a8
to
5fc0f3b
Compare
match str::parse::<u32>(etype_processed.as_str()) { | ||
Ok(value) => data.bit_len = Some(value), | ||
Err(_) => { | ||
data.bit_len = match data.kind() { | ||
TypeKind::Char(_) => Some(8), | ||
TypeKind::BFloat => Some(16), | ||
TypeKind::Int(_) => Some(32), | ||
TypeKind::Float => Some(32), | ||
_ => None, | ||
}; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are only some type kinds covered here? Maybe this could be a method on TypeKind
?
9e28106
to
111cd5d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you should rebase on top of the upstream master branch instead of merging it in. That keeps the git history clean.
x86_64-unknown-linux-gnu*) | ||
CPPFLAGS="${TEST_CPPFLAGS}" RUSTFLAGS="${HOST_RUSTFLAGS}" RUST_LOG=warn \ | ||
cargo run "${INTRINSIC_TEST}" "${PROFILE}" \ | ||
--bin intrinsic-test -- intrinsics_data/x86-intel.xml \ | ||
--runner "${TEST_RUNNER}" \ | ||
--cppcompiler "${TEST_CXX_COMPILER}" \ | ||
--target "${TARGET}" | ||
;; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we'll have to see how to do this exactly, but we want to split these out of the main CI job to speed it up
f3f87f2
to
2ec747c
Compare
2ec747c
to
a8313d0
Compare
Seems like the CI run at this point failed due to this error. I'll retry shortly:
|
a119be1
to
0f3900b
Compare
Notes: 1. chunk_info has been moved to `common/mod.rs` since it will be needed for all architectures
@folkertdev Seems like the I ran into an issue with the process being killed (likely due to resource constraints since the For now I've made it sequential so that I can step over that error, but the amount of time it's taking has got me reconsidering it. |
I don't think that is how it works? You just get a CI machine (typically with 4 cores) for some duration. Do you know how large these files get (how many intrinsics do you generate a test for)? We definitely want to be running these processes in parallel. |
Ohh I see, I thought I could edit the resource allocation. Let me experiment with keeping more number of |
you could also try to just cut the total number of generated tests (e.g. to 10%) to see if that helps? By the looks of it the whole process is just way too slow, and the runners eventually time out. So, you'll have to figure out how to make it faster. I think the crux here is what I plan to do: link all the c++ and rust code into one binary that generates, in effect, a large number of rust But that is probably not feasible for you in the remaining time, so by just cutting the total number of tests (maybe using some random sampling, e.g. keep every 10th test) we'll be able to merge your code without completely tanking our CI times. |
Good idea @folkertdev. Let me first check whether processing smaller-sized chunks would be of any help. |
c81c215
to
6733138
Compare
2876624
to
6be6472
Compare
6be6472
to
87e39a2
Compare
20ec01d
to
b476cd1
Compare
to C++ version of the same.
b476cd1
to
3b77dc5
Compare
ceb2a37
to
9dbc078
Compare
@sayantn was this change related to the change we made for some x86 intrinsics? Found this in the CI tests Illegal instruction at address = 56435cedbf49: f2 0f 2b 00 0f ae f8 f2 0f 10 04 24 66 0f 2e
Image name: /checkout/target/x86_64-unknown-linux-gnu/release/deps/core_arch-92d6a56b530b6fbf
Offset in image: 0x6cef49
If you believe your application should attempt to execute
this illegal instruction (and others that may be present),
Then use this knob: -emit-illegal-insts 0
and this error message will be avoided.
Use -print-exception-details to get more details.
SDE ERROR: Illegal instruction at address = 56435cedbf49: f2 0f 2b 00 0f ae f8 f2 0f 10 04 24 66 0f 2e
Image name: /checkout/target/x86_64-unknown-linux-gnu/release/deps/core_arch-92d6a56b530b6fbf
Offset in image: 0x6cef49
If you believe your application should attempt to execute
this illegal instruction (and others that may be present),
Then use this knob: -emit-illegal-insts 0
and this error message will be avoided.
Use -print-exception-details to get more details. |
I don't think I have seen this before, seems to be in |
81bd07f
to
8c19290
Compare
|
Yea, and the CI failures looked like it got to sse4a and then failed (although I don't know how much we can trust output order, considering that the tests are parallelized) |
SSE4a is an AMD extension, so intel-sde may not support it. Does the intrinsic tester check for feature availability before using those features? |
Iirc sde supports (most) amd extensions. It even has a cpu preset named our testsuite uses |
6c66eb7
to
9dbc078
Compare
Context
This is a redo of PR #1814, since a lot of details have changed with PRs #1863, #1862, #1861, #1852.
r? @folkertdev
cc: @Amanieu